refact: refactor format backends into dpdata.formats#970
Draft
njzjz-bot wants to merge 4 commits intodeepmodeling:masterfrom
Draft
refact: refactor format backends into dpdata.formats#970njzjz-bot wants to merge 4 commits intodeepmodeling:masterfrom
njzjz-bot wants to merge 4 commits intodeepmodeling:masterfrom
Conversation
dosubot
Bot
added
size:M
Merging this PR will improve performance by 22.65%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_import |
11.1 ms | 9.6 ms | +15.37% |
| ⚡ | WallTime | test_cli |
369.9 ms | 301.6 ms | +22.65% |
Comparing njzjz-bot:oc-fix-pr-946-ci (d6252fc) with master (6cdc360)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #970 +/- ##
==========================================
+ Coverage 86.73% 86.75% +0.01%
==========================================
Files 86 89 +3
Lines 8084 8093 +9
==========================================
+ Hits 7012 7021 +9
Misses 1072 1072 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis PR centralizes format-related modules under a new dpdata.formats package and updates import paths across the codebase to reference dpdata.formats.*. Package initializers for cp2k, gaussian, and qe were added and many relative imports adjusted for the deeper package nesting. ChangesModule reorganization into dpdata.formats (single cohesive DAG)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dpdata/bond_order_system.py (1)
81-91:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
data-only initialization still raisesValueErrordue to branch structureWhen
datais provided withoutfile_name/rdkit_mol, Line 81 initializes from data, but control still reaches theelseat Line 91 and raises. This breaks the documenteddatainit path.Proposed fix
- if data: + if data is not None: mol = dpdata.formats.rdkit.utils.system_data_to_mol(data) self.from_rdkit_mol(mol) - if file_name: + elif file_name: self.from_fmt( file_name, fmt, type_map=type_map, begin=begin, step=step, **kwargs ) - elif rdkit_mol: + elif rdkit_mol is not None: self.from_rdkit_mol(rdkit_mol) else: raise ValueError("Please specify a mol/sdf file or a rdkit Mol object")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dpdata/bond_order_system.py` around lines 81 - 91, The init path processes `data` but then continues into the file/rdkit branches and hits the final else, causing the erroneous ValueError; update the conditional flow in the BondOrderSystem initializer (or the method handling construction) so that the `data` case stops further branching—either change the `if data:` block to `if data: ... elif file_name: ... elif rdkit_mol: ... else: ...` or keep `if data:` and add an immediate return after calling from_rdkit_mol; ensure you reference the existing calls to dpdata.formats.rdkit.utils.system_data_to_mol, self.from_rdkit_mol, and self.from_fmt when making the change.dpdata/plugins/pymatgen.py (1)
72-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing import
dpdata.systemto fix runtime AttributeError.Line 72 uses
dpdata.system.remove_pbc(data), butdpdata.systemis not imported. Thedpdatapackage does not re-export thesystemmodule in its__init__.py—only individual classes from it are exposed. This will raiseAttributeErrorat runtime whento_systemis called on aPyMatgenMoleculeFormatinstance.Proposed fix — add explicit import
import dpdata.formats.pymatgen.molecule import dpdata.formats.pymatgen.structure +import dpdata.system from dpdata.format import Format🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dpdata/plugins/pymatgen.py` at line 72, The call data = dpdata.system.remove_pbc(data) fails at runtime because the dpdata.system submodule isn't imported; add an explicit import (e.g., import dpdata.system) at the top of the file and then leave the call in PyMatgenMoleculeFormat.to_system as-is so dpdata.system.remove_pbc(data) resolves correctly.
🧹 Nitpick comments (1)
dpdata/plugins/xyz.py (1)
11-14: ⚡ Quick winRuntime imports placed after the
if TYPE_CHECKING:block — invert the order.Lines 13–14 are unconditional runtime imports but appear after the
if TYPE_CHECKING:guard. The conventional (and ruff/isort-expected) layout places theif TYPE_CHECKING:block last among all imports. This ordering may trigger anI001violation depending on the project's ruff configuration.♻️ Proposed fix
+from dpdata.formats.xyz.quip_gap_xyz import QuipGapxyzSystems, format_single_frame +from dpdata.formats.xyz.xyz import coord_to_xyz, xyz_to_coord + if TYPE_CHECKING: from dpdata.utils import FileType -from dpdata.formats.xyz.quip_gap_xyz import QuipGapxyzSystems, format_single_frame -from dpdata.formats.xyz.xyz import coord_to_xyz, xyz_to_coordAs per coding guidelines,
dpdata/**/*.pyfiles must passruff check dpdata/before committing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dpdata/plugins/xyz.py` around lines 11 - 14, The import ordering is wrong: move the TYPE_CHECKING block so it comes after the runtime imports (i.e., place the "if TYPE_CHECKING: from dpdata.utils import FileType" block below the imports of QuipGapxyzSystems, format_single_frame, coord_to_xyz, and xyz_to_coord) to satisfy ruff/isort expectations and avoid I001; ensure the runtime symbols QuipGapxyzSystems, format_single_frame, coord_to_xyz, and xyz_to_coord remain imported unconditionally and only FileType is guarded by TYPE_CHECKING.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dpdata/__init__.py`:
- Line 4: Run ruff check and fix the reported lint issues: sort the __all__
lists in dpdata/__init__ and dpdata/formats/deepmd/hdf5.py, replace mutable
default args/attributes in functions/classes (e.g., in ase_calculator.py,
rdf.py, water.py) with None and set defaults inside the function or __init__,
add explicit stacklevel=2 to all warnings.warn calls, rename variables shadowing
builtins (e.g., in driver.py, hdf5.py and pwmat-related files), remove or use
unused loop control variables (abacus, cp2k, fhi_aims, gromacs, lammps, md,
openmx, pwmat modules) or replace with _ if intentionally unused, add strict=...
to zip() calls, and address the remaining RUF/B/BLE/etc. issues (unused
unpacking, unnecessary conversions/concatenations, empty abstract methods,
assert False, missing shebangs, ambiguous names, overly broad exception
handlers) as indicated by ruff to make the codebase clean.
In `@dpdata/plugins/openmx.py`:
- Line 64: The unpacked variable `cs` from the call to
dpdata.formats.openmx.omx.to_system_data(fname, mdname) is unused and causes a
Ruff RUF059 lint error; update the unpack to either capture the unused value as
`_` (e.g., `data, _ = ...`) or assign only `data` (e.g., `data = ...`) inside
openmx.py where the call occurs, and then run ruff check dpdata/ and ruff format
dpdata/ to ensure linting/formatting compliance.
---
Outside diff comments:
In `@dpdata/bond_order_system.py`:
- Around line 81-91: The init path processes `data` but then continues into the
file/rdkit branches and hits the final else, causing the erroneous ValueError;
update the conditional flow in the BondOrderSystem initializer (or the method
handling construction) so that the `data` case stops further branching—either
change the `if data:` block to `if data: ... elif file_name: ... elif rdkit_mol:
... else: ...` or keep `if data:` and add an immediate return after calling
from_rdkit_mol; ensure you reference the existing calls to
dpdata.formats.rdkit.utils.system_data_to_mol, self.from_rdkit_mol, and
self.from_fmt when making the change.
In `@dpdata/plugins/pymatgen.py`:
- Line 72: The call data = dpdata.system.remove_pbc(data) fails at runtime
because the dpdata.system submodule isn't imported; add an explicit import
(e.g., import dpdata.system) at the top of the file and then leave the call in
PyMatgenMoleculeFormat.to_system as-is so dpdata.system.remove_pbc(data)
resolves correctly.
---
Nitpick comments:
In `@dpdata/plugins/xyz.py`:
- Around line 11-14: The import ordering is wrong: move the TYPE_CHECKING block
so it comes after the runtime imports (i.e., place the "if TYPE_CHECKING: from
dpdata.utils import FileType" block below the imports of QuipGapxyzSystems,
format_single_frame, coord_to_xyz, and xyz_to_coord) to satisfy ruff/isort
expectations and avoid I001; ensure the runtime symbols QuipGapxyzSystems,
format_single_frame, coord_to_xyz, and xyz_to_coord remain imported
unconditionally and only FileType is guarded by TYPE_CHECKING.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d56fd68-bd9e-45f8-b025-b4160b3f00b9
📒 Files selected for processing (99)
dpdata/__init__.pydpdata/bond_order_system.pydpdata/formats/__init__.pydpdata/formats/abacus/__init__.pydpdata/formats/abacus/md.pydpdata/formats/abacus/relax.pydpdata/formats/abacus/scf.pydpdata/formats/abacus/stru.pydpdata/formats/amber/__init__.pydpdata/formats/amber/mask.pydpdata/formats/amber/md.pydpdata/formats/amber/sqm.pydpdata/formats/cp2k/__init__.pydpdata/formats/cp2k/cell.pydpdata/formats/cp2k/output.pydpdata/formats/deepmd/__init__.pydpdata/formats/deepmd/comp.pydpdata/formats/deepmd/hdf5.pydpdata/formats/deepmd/mixed.pydpdata/formats/deepmd/raw.pydpdata/formats/dftbplus/__init__.pydpdata/formats/dftbplus/output.pydpdata/formats/fhi_aims/__init__.pydpdata/formats/fhi_aims/output.pydpdata/formats/gaussian/__init__.pydpdata/formats/gaussian/fchk.pydpdata/formats/gaussian/gjf.pydpdata/formats/gaussian/log.pydpdata/formats/gromacs/__init__.pydpdata/formats/gromacs/gro.pydpdata/formats/lammps/__init__.pydpdata/formats/lammps/dump.pydpdata/formats/lammps/lmp.pydpdata/formats/lmdb/__init__.pydpdata/formats/lmdb/format.pydpdata/formats/md/__init__.pydpdata/formats/md/msd.pydpdata/formats/md/pbc.pydpdata/formats/md/rdf.pydpdata/formats/md/water.pydpdata/formats/openmx/__init__.pydpdata/formats/openmx/omx.pydpdata/formats/orca/__init__.pydpdata/formats/orca/output.pydpdata/formats/psi4/__init__.pydpdata/formats/psi4/input.pydpdata/formats/psi4/output.pydpdata/formats/pwmat/__init__.pydpdata/formats/pwmat/atomconfig.pydpdata/formats/pwmat/movement.pydpdata/formats/pymatgen/__init__.pydpdata/formats/pymatgen/molecule.pydpdata/formats/pymatgen/structure.pydpdata/formats/qe/__init__.pydpdata/formats/qe/scf.pydpdata/formats/qe/traj.pydpdata/formats/rdkit/__init__.pydpdata/formats/rdkit/sanitize.pydpdata/formats/rdkit/utils.pydpdata/formats/siesta/__init__.pydpdata/formats/siesta/aiMD_output.pydpdata/formats/siesta/output.pydpdata/formats/vasp/__init__.pydpdata/formats/vasp/outcar.pydpdata/formats/vasp/poscar.pydpdata/formats/vasp/xml.pydpdata/formats/xyz/__init__.pydpdata/formats/xyz/quip_gap_xyz.pydpdata/formats/xyz/xyz.pydpdata/plugins/3dmol.pydpdata/plugins/abacus.pydpdata/plugins/amber.pydpdata/plugins/cp2k.pydpdata/plugins/deepmd.pydpdata/plugins/dftbplus.pydpdata/plugins/fhi_aims.pydpdata/plugins/gaussian.pydpdata/plugins/gromacs.pydpdata/plugins/lammps.pydpdata/plugins/lmdb.pydpdata/plugins/openmx.pydpdata/plugins/orca.pydpdata/plugins/psi4.pydpdata/plugins/pwmat.pydpdata/plugins/pymatgen.pydpdata/plugins/qe.pydpdata/plugins/rdkit.pydpdata/plugins/siesta.pydpdata/plugins/vasp.pydpdata/plugins/xyz.pydpdata/siesta/__init__.pydpdata/system.pydpdata/vasp/__init__.pydpdata/xyz/__init__.pytests/context.pytests/test_abacus_stru_dump.pytests/test_lammps_lmp_dump.pytests/test_lammps_spin.pytests/test_lmdb.py
njzjz-bot
force-pushed
the
oc-fix-pr-946-ci
branch
4 times, most recently
from
4c16ae8 to
fda1705
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR rebases the prior directory reorganization (moving format implementations under dpdata.formats) and updates internal imports/tests accordingly, with the stated goal of restoring backward-compatible access to historically public helper modules/namespaces after the move.
Changes:
- Relocates/introduces many format implementation modules under
dpdata/formats/**(e.g., VASP/LAMMPS/QE/Gaussian/CP2K/LMDB, etc.). - Updates plugin modules and some core code to import from
dpdata.formats.*instead of historical locations. - Adds a lazy attribute-based loader in
dpdata/__init__.pyfor some format modules (cp2k,gaussian,qe).
Reviewed changes
Copilot reviewed 44 out of 99 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
dpdata/__init__.py |
Switches top-level exports to dpdata.formats.* and adds lazy __getattr__ for some format modules. |
dpdata/system.py |
Updates internal imports to dpdata.formats.md.pbc and dpdata.formats.amber.mask. |
dpdata/bond_order_system.py |
Updates RDKit helpers import path to dpdata.formats.rdkit.*. |
dpdata/plugins/3dmol.py |
Updates XYZ helper import path to dpdata.formats.xyz.xyz. |
dpdata/plugins/abacus.py |
Points ABACUS plugin imports to dpdata.formats.abacus.*. |
dpdata/plugins/amber.py |
Points Amber plugin imports to dpdata.formats.amber.*. |
dpdata/plugins/cp2k.py |
Points CP2K plugin imports to dpdata.formats.cp2k.output. |
dpdata/plugins/deepmd.py |
Points DeePMD plugin imports to dpdata.formats.deepmd.*. |
dpdata/plugins/dftbplus.py |
Points DFTB+ plugin imports to dpdata.formats.dftbplus.output. |
dpdata/plugins/fhi_aims.py |
Points FHI-aims plugin imports to dpdata.formats.fhi_aims.output. |
dpdata/plugins/gaussian.py |
Points Gaussian plugin imports to dpdata.formats.gaussian.* and updates doc references. |
dpdata/plugins/gromacs.py |
Points Gromacs plugin imports to dpdata.formats.gromacs.gro. |
dpdata/plugins/lammps.py |
Points LAMMPS plugin imports to dpdata.formats.lammps.*. |
dpdata/plugins/lmdb.py |
Registers LMDB format via dpdata.formats.lmdb.format.LMDBFormat. |
dpdata/plugins/openmx.py |
Points OpenMX plugin imports to dpdata.formats.openmx.* and dpdata.formats.md.pbc. |
dpdata/plugins/orca.py |
Points ORCA plugin imports to dpdata.formats.orca.output. |
dpdata/plugins/psi4.py |
Points Psi4 plugin imports to dpdata.formats.psi4.*. |
dpdata/plugins/pwmat.py |
Points PWMAT plugin imports to dpdata.formats.pwmat.*. |
dpdata/plugins/pymatgen.py |
Points pymatgen plugin imports to dpdata.formats.pymatgen.*. |
dpdata/plugins/qe.py |
Points QE plugin imports to dpdata.formats.qe.* and dpdata.formats.md.pbc. |
dpdata/plugins/rdkit.py |
Points RDKit plugin imports to dpdata.formats.rdkit.utils. |
dpdata/plugins/siesta.py |
Points SIESTA plugin imports to dpdata.formats.siesta.*. |
dpdata/plugins/vasp.py |
Points VASP plugin imports to dpdata.formats.vasp.*. |
dpdata/plugins/xyz.py |
Points XYZ plugin imports to dpdata.formats.xyz.*. |
dpdata/formats/__init__.py |
Introduces the dpdata.formats package marker. |
dpdata/formats/abacus/md.py |
Adds/relocates ABACUS MD reader under formats. |
dpdata/formats/abacus/relax.py |
Adds/relocates ABACUS relax reader under formats. |
dpdata/formats/abacus/scf.py |
Adjusts imports for ABACUS SCF under formats. |
dpdata/formats/abacus/stru.py |
Adjusts imports for ABACUS STRU under formats. |
dpdata/formats/amber/mask.py |
Adds/relocates Amber mask utilities under formats. |
dpdata/formats/amber/md.py |
Adjusts imports for Amber MD under formats. |
dpdata/formats/amber/sqm.py |
Adds/relocates SQM parsing/input generation under formats. |
dpdata/formats/cp2k/cell.py |
Adds/relocates CP2K cell helper under formats. |
dpdata/formats/cp2k/output.py |
Adjusts imports for CP2K output reader under formats. |
dpdata/formats/deepmd/comp.py |
Adds/relocates deepmd/npy (“comp”) support under formats. |
dpdata/formats/deepmd/hdf5.py |
Adds/relocates deepmd/hdf5 support under formats. |
dpdata/formats/deepmd/mixed.py |
Adds/relocates deepmd mixed-type utilities under formats. |
dpdata/formats/deepmd/raw.py |
Adds/relocates deepmd/raw support under formats. |
dpdata/formats/dftbplus/output.py |
Adds/relocates DFTB+ output reader under formats. |
dpdata/formats/fhi_aims/output.py |
Adds/relocates FHI-aims output reader under formats. |
dpdata/formats/gaussian/fchk.py |
Adjusts relative imports for Gaussian fchk under formats. |
dpdata/formats/gaussian/gjf.py |
Adds/relocates Gaussian input generator/parser under formats. |
dpdata/formats/gaussian/log.py |
Adjusts relative imports for Gaussian log under formats. |
dpdata/formats/gromacs/gro.py |
Adjusts relative imports for Gromacs gro under formats. |
dpdata/formats/lammps/dump.py |
Adds/relocates LAMMPS dump parsing/writing under formats. |
dpdata/formats/lammps/lmp.py |
Adds/relocates LAMMPS data-file parsing/writing under formats. |
dpdata/formats/lmdb/format.py |
Adds/relocates LMDB format implementation under formats. |
dpdata/formats/md/msd.py |
Adds/relocates MSD implementation under formats. |
dpdata/formats/md/pbc.py |
Adds/relocates PBC utilities under formats. |
dpdata/formats/md/rdf.py |
Adds/relocates RDF implementation under formats. |
dpdata/formats/md/water.py |
Adds/relocates water analysis utilities under formats. |
dpdata/formats/openmx/omx.py |
Adjusts relative imports for OpenMX under formats. |
dpdata/formats/orca/output.py |
Adds/relocates ORCA output reader under formats. |
dpdata/formats/psi4/input.py |
Adds/relocates Psi4 input writer under formats. |
dpdata/formats/psi4/output.py |
Adds/relocates Psi4 output reader under formats. |
dpdata/formats/pwmat/atomconfig.py |
Adjusts relative imports for PWMAT atomconfig under formats. |
dpdata/formats/pwmat/movement.py |
Adjusts relative imports for PWMAT movement under formats. |
dpdata/formats/pymatgen/molecule.py |
Adds/relocates pymatgen Molecule conversion under formats. |
dpdata/formats/pymatgen/structure.py |
Adds/relocates pymatgen Structure conversion under formats. |
dpdata/formats/qe/scf.py |
Adds/relocates QE SCF parsing under formats. |
dpdata/formats/qe/traj.py |
Fixes relative imports within QE traj under formats. |
dpdata/formats/rdkit/utils.py |
Adds/relocates RDKit helper utilities under formats. |
dpdata/formats/siesta/aiMD_output.py |
Adds/relocates SIESTA aiMD output reader under formats. |
dpdata/formats/siesta/output.py |
Adds/relocates SIESTA output reader under formats. |
dpdata/formats/vasp/outcar.py |
Adds/relocates VASP OUTCAR parsing under formats. |
dpdata/formats/vasp/poscar.py |
Adds/relocates VASP POSCAR parsing/writing under formats. |
dpdata/formats/vasp/xml.py |
Adds/relocates VASP XML parsing under formats. |
dpdata/formats/xyz/quip_gap_xyz.py |
Adds/relocates QUIP/GAP XYZ support under formats. |
dpdata/formats/xyz/xyz.py |
Adds/relocates basic XYZ conversions under formats. |
tests/context.py |
Updates import smoke-loading to dpdata.formats.*. |
tests/test_abacus_stru_dump.py |
Updates ABACUS test imports to dpdata.formats.abacus.*. |
tests/test_lammps_lmp_dump.py |
Updates LAMMPS test imports to dpdata.formats.lammps.*. |
tests/test_lammps_spin.py |
Updates LAMMPS test imports to dpdata.formats.lammps.*. |
tests/test_lmdb.py |
Updates LMDB test imports to dpdata.formats.lmdb.*. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
njzjz-bot
force-pushed
the
oc-fix-pr-946-ci
branch
3 times, most recently
from
92a9266 to
e23e05e
Compare
njzjz-bot
changed the title
njzjz-bot
force-pushed
the
oc-fix-pr-946-ci
branch
2 times, most recently
from
2482d02 to
ae36482
Compare
njzjz-bot
changed the title
njzjz
changed the title
Contributor
|
conflicts should be resolved. |
Move all format directories (abacus, amber, cp2k, deepmd, dftbplus, fhi_aims, gaussian, gromacs, lammps, lmdb, md, openmx, orca, psi4, pwmat, pymatgen, qe, rdkit, siesta, vasp, xyz) into a new formats/ subdirectory. This addresses issue deepmodeling#934. Changes: - Created dpdata/formats/ directory - Moved all format directories to dpdata/formats/ - Updated all import statements throughout the codebase - Updated relative imports in format modules (from .. to from ...) - Updated dpdata/__init__.py to import from new locations - Updated tests/context.py for new import paths The plugins directory remains at the root level as requested.
for more information, see https://pre-commit.ci
njzjz-bot
force-pushed
the
oc-fix-pr-946-ci
branch
from
ae36482 to
9680f7c
Compare
dosubot
Bot
removed
the
size:M
dosubot
Bot
added
the
size:XXL
Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
njzjz-bot
force-pushed
the
oc-fix-pr-946-ci
branch
from
cfdb266 to
d6252fc
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dpdata/plugins/xyz.py (1)
1-117: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRun mandated Ruff checks before merge.
Please confirm
ruff check dpdata/andruff format dpdata/were run for this PR branch before merging.As per coding guidelines, "Run ruff linting with
ruff check dpdata/and format code withruff format dpdata/before committing".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dpdata/plugins/xyz.py` around lines 1 - 117, Run the repository linter/formatter on the dpdata package and commit any fixes: execute "ruff check dpdata/" to identify issues and "ruff format dpdata/" to auto-format, then re-run "ruff check dpdata/" to confirm there are no remaining offenses; ensure changes (especially in the modified symbols/classes like XYZFormat and QuipGapXYZFormat in dpdata/plugins/xyz.py and any related imports such as coord_to_xyz, xyz_to_coord, QuipGapxyzSystems, format_single_frame) are staged and committed before merging.
♻️ Duplicate comments (1)
dpdata/plugins/openmx.py (1)
64-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused unpacked variable to satisfy lint.
The variable
csis assigned but never used, triggering RuffRUF059. This should use_instead, matching the pattern already used on Line 38.🔧 Proposed fix
- data, cs = dpdata.formats.openmx.omx.to_system_data(fname, mdname) + data, _ = dpdata.formats.openmx.omx.to_system_data(fname, mdname)As per coding guidelines,
dpdata/**/*.py: Run ruff linting withruff check dpdata/and format code withruff format dpdata/before committing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dpdata/plugins/openmx.py` at line 64, The assignment unpacks two values from dpdata.formats.openmx.omx.to_system_data(fname, mdname) into variables `data, cs` but `cs` is unused and triggers RUF059; change the unused unpacked variable `cs` to `_` (i.e., `data, _ = dpdata.formats.openmx.omx.to_system_data(fname, mdname)`) in the dpdata.plugins.openmx module and then run ruff check/format on the dpdata/ package as per guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@dpdata/plugins/xyz.py`:
- Around line 1-117: Run the repository linter/formatter on the dpdata package
and commit any fixes: execute "ruff check dpdata/" to identify issues and "ruff
format dpdata/" to auto-format, then re-run "ruff check dpdata/" to confirm
there are no remaining offenses; ensure changes (especially in the modified
symbols/classes like XYZFormat and QuipGapXYZFormat in dpdata/plugins/xyz.py and
any related imports such as coord_to_xyz, xyz_to_coord, QuipGapxyzSystems,
format_single_frame) are staged and committed before merging.
---
Duplicate comments:
In `@dpdata/plugins/openmx.py`:
- Line 64: The assignment unpacks two values from
dpdata.formats.openmx.omx.to_system_data(fname, mdname) into variables `data,
cs` but `cs` is unused and triggers RUF059; change the unused unpacked variable
`cs` to `_` (i.e., `data, _ = dpdata.formats.openmx.omx.to_system_data(fname,
mdname)`) in the dpdata.plugins.openmx module and then run ruff check/format on
the dpdata/ package as per guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b2ea20e0-3772-4448-8e73-4cff8fb7f254
📒 Files selected for processing (100)
dpdata/__init__.pydpdata/bond_order_system.pydpdata/formats/__init__.pydpdata/formats/abacus/__init__.pydpdata/formats/abacus/md.pydpdata/formats/abacus/relax.pydpdata/formats/abacus/scf.pydpdata/formats/abacus/stru.pydpdata/formats/amber/__init__.pydpdata/formats/amber/mask.pydpdata/formats/amber/md.pydpdata/formats/amber/sqm.pydpdata/formats/cp2k/__init__.pydpdata/formats/cp2k/cell.pydpdata/formats/cp2k/output.pydpdata/formats/deepmd/__init__.pydpdata/formats/deepmd/comp.pydpdata/formats/deepmd/hdf5.pydpdata/formats/deepmd/mixed.pydpdata/formats/deepmd/raw.pydpdata/formats/dftbplus/__init__.pydpdata/formats/dftbplus/output.pydpdata/formats/fhi_aims/__init__.pydpdata/formats/fhi_aims/output.pydpdata/formats/gaussian/__init__.pydpdata/formats/gaussian/fchk.pydpdata/formats/gaussian/gjf.pydpdata/formats/gaussian/log.pydpdata/formats/gromacs/__init__.pydpdata/formats/gromacs/gro.pydpdata/formats/lammps/__init__.pydpdata/formats/lammps/dump.pydpdata/formats/lammps/lmp.pydpdata/formats/lmdb/__init__.pydpdata/formats/lmdb/format.pydpdata/formats/openmx/__init__.pydpdata/formats/openmx/omx.pydpdata/formats/orca/__init__.pydpdata/formats/orca/output.pydpdata/formats/psi4/__init__.pydpdata/formats/psi4/input.pydpdata/formats/psi4/output.pydpdata/formats/pwmat/__init__.pydpdata/formats/pwmat/atomconfig.pydpdata/formats/pwmat/movement.pydpdata/formats/pymatgen/__init__.pydpdata/formats/pymatgen/molecule.pydpdata/formats/pymatgen/structure.pydpdata/formats/qe/__init__.pydpdata/formats/qe/scf.pydpdata/formats/qe/traj.pydpdata/formats/rdkit/__init__.pydpdata/formats/rdkit/sanitize.pydpdata/formats/rdkit/utils.pydpdata/formats/siesta/__init__.pydpdata/formats/siesta/aiMD_output.pydpdata/formats/siesta/output.pydpdata/formats/vasp/__init__.pydpdata/formats/vasp/outcar.pydpdata/formats/vasp/poscar.pydpdata/formats/vasp/xml.pydpdata/formats/xyz/__init__.pydpdata/formats/xyz/quip_gap_xyz.pydpdata/formats/xyz/xyz.pydpdata/plugins/3dmol.pydpdata/plugins/abacus.pydpdata/plugins/amber.pydpdata/plugins/cp2k.pydpdata/plugins/deepmd.pydpdata/plugins/dftbplus.pydpdata/plugins/fhi_aims.pydpdata/plugins/gaussian.pydpdata/plugins/gromacs.pydpdata/plugins/lammps.pydpdata/plugins/lmdb.pydpdata/plugins/openmx.pydpdata/plugins/orca.pydpdata/plugins/psi4.pydpdata/plugins/pwmat.pydpdata/plugins/pymatgen.pydpdata/plugins/qe.pydpdata/plugins/rdkit.pydpdata/plugins/siesta.pydpdata/plugins/vasp.pydpdata/plugins/xyz.pydpdata/siesta/__init__.pydpdata/system.pydpdata/vasp/__init__.pydpdata/xyz/__init__.pytests/context.pytests/test_abacus_stru_dump.pytests/test_amber_md.pytests/test_cell_to_low_triangle.pytests/test_gaussian_driver.pytests/test_lammps_lmp_dump.pytests/test_lammps_spin.pytests/test_lmdb.pytests/test_msd.pytests/test_qe_cp_traj.pytests/test_water_ions.py
✅ Files skipped from review due to trivial changes (20)
- dpdata/formats/pwmat/movement.py
- dpdata/formats/init.py
- dpdata/formats/gromacs/gro.py
- dpdata/formats/abacus/scf.py
- dpdata/plugins/lmdb.py
- dpdata/plugins/3dmol.py
- tests/test_lmdb.py
- dpdata/formats/gaussian/log.py
- tests/test_amber_md.py
- dpdata/formats/cp2k/output.py
- dpdata/plugins/dftbplus.py
- dpdata/formats/qe/init.py
- dpdata/plugins/psi4.py
- dpdata/formats/gaussian/init.py
- dpdata/formats/cp2k/init.py
- tests/test_qe_cp_traj.py
- dpdata/plugins/cp2k.py
- dpdata/plugins/lammps.py
- dpdata/plugins/rdkit.py
- tests/test_water_ions.py
🚧 Files skipped from review as they are similar to previous changes (21)
- dpdata/formats/pwmat/atomconfig.py
- tests/test_lammps_lmp_dump.py
- dpdata/formats/amber/md.py
- dpdata/formats/abacus/stru.py
- dpdata/formats/qe/traj.py
- tests/test_msd.py
- dpdata/formats/gaussian/fchk.py
- dpdata/plugins/orca.py
- tests/test_cell_to_low_triangle.py
- tests/context.py
- dpdata/plugins/fhi_aims.py
- dpdata/plugins/gromacs.py
- dpdata/plugins/siesta.py
- tests/test_abacus_stru_dump.py
- dpdata/plugins/vasp.py
- dpdata/formats/openmx/omx.py
- dpdata/system.py
- dpdata/plugins/deepmd.py
- dpdata/plugins/gaussian.py
- dpdata/plugins/amber.py
- dpdata/plugins/qe.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the CI failures from #946 after moving implementation modules.
Changes:
dpdata.formats.dpdata.mdinstead ofdpdata.formats.md.dpdata.lammps/dpdata.vaspas top-level exports.dpdata.formats.dpdata.formats.cp2k.cell.cell_to_low_triangledpdata.formats.gaussian.gjf.detect_multiplicitydpdata.formats.qe.traj.convert_celldmdpdata.formats.amber.md.cell_lengths_angles_to_celldpdata.md.msd.msddpdata.md.water.*Follow-up:
dpdata.<format>wrapper modules that were added earlier; this branch no longer keeps those old import paths alive.Local checks:
cd tests && uv run pytest test_amber_md.py test_cell_to_low_triangle.py test_gaussian_driver.py::TestMakeGaussian::test_detect_multiplicity test_qe_cp_traj.py::TestConverCellDim test_msd.py test_water_ions.py -q→ 45 passeduv run pyright→ currently reports 2 pre-existing missing_version/__version__diagnostics indpdata/__init__.pyanddpdata/cli.pygit grep -n "from dpdata\.formats\..* import \*\|legacy" -- dpdata tests→ no matchesAuthored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
Summary by CodeRabbit
Release Notes
formatssubdirectory for improved code structure and maintainability.